-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jupyter: Restore terminal and allow recursive folder deletion #280
Jupyter: Restore terminal and allow recursive folder deletion #280
Conversation
It was disabled to avoid malicious usage but with the monitoring in place and the demo account restricted to limited resources, it's probably safe to try enabling this again. For legitimate users, not having the terminal is pretty annoying. Should not penalize legit users for some rogue users.
This was not possible before since non-empty dir deletion was not possible.
The container died instantly on creation startup.
Fix this error found during autodeploy: ``` fix GeoServer data dir permission on first run only, when data dir do not exist yet. + DATA_DIR='${DATA_PERSIST_ROOT}/geoserver' + '[' -n ] + docker run --rm --name fix-geoserver-data-dir-perm --volume '${DATA_PERSIST_ROOT}/geoserver:/datadir' --env FIRST_RUN_ONLY bash:5.1.4 bash -xc 'if [ -z "$FIRST_RUN_ONLY" -o ! -f /datadir/global.xml ]; \ then chown -R 1000:10001 /datadir; else echo "No execute."; fi' docker: Error response from daemon: create ${DATA_PERSIST_ROOT}/geoserver: "${DATA_PERSIST_ROOT}/geoserver" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path. See 'docker run --help'. ```
…lways read last env.local will always have the latest updated values so no need for delayed eval.
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/35/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/925/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1140/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/925/NOTEBOOK TEST RESULTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look OK, but I wonder if it would not be better to have a separate .sh
that does the full process_delayed_eval
definition + . env.local
+ . default.env
steps in the appropriate order. In other words, move the logic of sourcing ENV definitions together instead of duplicated in each script. It will be easier to maintain in case other approaches like in #272 (comment) are attempted.
Agreed. I think a further "logic grouping" is required. Retrospectively, maybe I should not have supported delayed eval in the first place. It's a bit more complex than initially planned for not a very big gain (only 3 variables so far). So I am not sure I want to put more effort into this feature. Let's keep it as-is for now and see how it evolves. But you are right, next time we leverage this feature again and decide to keep this feature, the "logic grouping" should be done at the same time. |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1142/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/927/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/37/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/927/NOTEBOOK TEST RESULTS |
I think it would be better to group them right away, but without modifying the DELAYED_VARS approach yet. If this new method is applied right away and spreads across scripts (including user-specific overrides), it will have an even bigger impact with more restrictions to modify it later. |
Then I'll remove the fix in this PR and open a different PR to avoid delaying this PR. |
This reverts commit 6687185. To be implemented better in a different PR. Francis proposed to centralize all the sourcing logic of default.env and env.local and calling process_delayed_eval in a same file so all scripts to not have to remember the loading sequence and we have a central location to update that loading sequence without having to edit all the scripts in the future.
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1143/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/928/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/38/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/929/NOTEBOOK TEST RESULTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok with scope change.
Looking at the nb failures.
|
I think it's because it looks for the Must be a precommit autoformat that breaks that line on multple lines. But I don't understand, I thought we escaped all the lines with
Seems to be test env configuration problem. User
|
Merged this PR anyways since the 2 failed notebooks are absolutely unrelated to my JupyterHub config change. |
The escape for the precommit autoformater to avoid wrapping the line was already done in this commit: Ouranosinc/pavics-sdi@75652fb Now sure how this escape got lost. I'll just restore it. |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1144/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL :NOTEBOOK TEST RESULTS
|
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac/39/Result : failure BIRDHOUSE_DEPLOY_BRANCH : restore-jupyter-terminal-and-allow-recursive-folder-deletion DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL :NOTEBOOK TEST RESULTS
|
…277) Should have used the climex link on PAVICS prod server instead of the test server. `TEST_USE_PROD_DATA` should be on the same line as "pavics.ouranos.ca" to avoid it being replaced by the test server during Jenkins run. The precommit autoformatter should not break that line. See previous fix that was lost, not sure why 75652fb (PR #267) Jenkins failure found in PR bird-house/birdhouse-deploy#280 (comment): ``` 10:36:34 _________ pavics-sdi-master/docs/source/notebooks/climex.ipynb::Cell 0 _________ 10:36:34 Notebook cell execution failed 10:36:34 Cell 0: Cell execution caused an exception 10:36:34 10:36:34 Input: 10:36:34 import shutil 10:36:34 10:36:34 import intake 10:36:34 import xarray as xr 10:36:34 import xclim 10:36:34 from clisops.core.subset import subset_gridpoint 10:36:34 from dask.diagnostics import ProgressBar 10:36:34 from dask.distributed import Client, LocalCluster 10:36:34 from IPython.display import HTML, Markdown 10:36:34 from xclim import ensembles as xens 10:36:34 10:36:34 cat = intake.open_esm_datastore( 10:36:34 "https://host-140-154.rdext.crim.ca/catalog/climex.json" 10:36:34 ) # TEST_USE_PROD_DATA 10:36:34 cat.df.head() 10:36:34 10:36:34 Traceback: 10:36:34 10:36:34 --------------------------------------------------------------------------- 10:36:34 JSONDecodeError Traceback (most recent call last) 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/requests/models.py:971, in Response.json(self, **kwargs) 10:36:34 970 try: 10:36:34 --> 971 return complexjson.loads(self.text, **kwargs) 10:36:34 972 except JSONDecodeError as e: 10:36:34 973 # Catch JSON-related errors and raise as requests.JSONDecodeError 10:36:34 974 # This aliases json.JSONDecodeError and simplejson.JSONDecodeError 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/json/__init__.py:357, in loads(s, cls, object_hook, parse_float, parse_int, parse_constant, object_pairs_hook, **kw) 10:36:34 354 if (cls is None and object_hook is None and 10:36:34 355 parse_int is None and parse_float is None and 10:36:34 356 parse_constant is None and object_pairs_hook is None and not kw): 10:36:34 --> 357 return _default_decoder.decode(s) 10:36:34 358 if cls is None: 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/json/decoder.py:337, in JSONDecoder.decode(self, s, _w) 10:36:34 333 """Return the Python representation of ``s`` (a ``str`` instance 10:36:34 334 containing a JSON document). 10:36:34 335 10:36:34 336 """ 10:36:34 --> 337 obj, end = self.raw_decode(s, idx=_w(s, 0).end()) 10:36:34 338 end = _w(s, end).end() 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/json/decoder.py:355, in JSONDecoder.raw_decode(self, s, idx) 10:36:34 354 except StopIteration as err: 10:36:34 --> 355 raise JSONDecodeError("Expecting value", s, err.value) from None 10:36:34 356 return obj, end 10:36:34 10:36:34 JSONDecodeError: Expecting value: line 4 column 1 (char 3) 10:36:34 10:36:34 During handling of the above exception, another exception occurred: 10:36:34 10:36:34 JSONDecodeError Traceback (most recent call last) 10:36:34 Cell In [1], line 12 10:36:34 9 from IPython.display import HTML, Markdown 10:36:34 10 from xclim import ensembles as xens 10:36:34 ---> 12 cat = intake.open_esm_datastore( 10:36:34 13 "https://host-140-154.rdext.crim.ca/catalog/climex.json" 10:36:34 14 ) # TEST_USE_PROD_DATA 10:36:34 15 cat.df.head() 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/intake_esm/core.py:83, in esm_datastore.__init__(self, esmcol_obj, esmcol_data, progressbar, sep, csv_kwargs, **kwargs) 10:36:34 81 super(esm_datastore, self).__init__(**kwargs) 10:36:34 82 if isinstance(esmcol_obj, (str, pathlib.PurePath)): 10:36:34 ---> 83 self.esmcol_data, self.esmcol_path = _fetch_and_parse_json(esmcol_obj) 10:36:34 84 self._df, self.catalog_file = _fetch_catalog(self.esmcol_data, esmcol_obj, csv_kwargs) 10:36:34 86 elif isinstance(esmcol_obj, pd.DataFrame): 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/intake_esm/utils.py:59, in _fetch_and_parse_json(input_path) 10:36:34 56 data = json.load(filein) 10:36:34 58 except Exception as exc: 10:36:34 ---> 59 raise exc 10:36:34 61 return data, input_path 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/intake_esm/utils.py:52, in _fetch_and_parse_json(input_path) 10:36:34 50 if _is_valid_url(input_path): 10:36:34 51 resp = requests.get(input_path) 10:36:34 ---> 52 data = resp.json() 10:36:34 53 else: 10:36:34 54 input_path = Path(input_path).absolute().as_posix() 10:36:34 10:36:34 File /opt/conda/envs/birdy/lib/python3.8/site-packages/requests/models.py:975, in Response.json(self, **kwargs) 10:36:34 971 return complexjson.loads(self.text, **kwargs) 10:36:34 972 except JSONDecodeError as e: 10:36:34 973 # Catch JSON-related errors and raise as requests.JSONDecodeError 10:36:34 974 # This aliases json.JSONDecodeError and simplejson.JSONDecodeError 10:36:34 --> 975 raise RequestsJSONDecodeError(e.msg, e.doc, e.pos) 10:36:34 10:36:34 JSONDecodeError: Expecting value: line 4 column 1 (char 3) ```
Changes:
Jupyter: allow recursive directory deletion
This was not possible before since non-empty dir deletion was not possible.
Jupyter: re-enable terminal for all users
It was disabled to avoid malicious usage but with the monitoring in
place and the demo account restricted to limited resources, it's
probably safe to try enabling this again.
For legitimate users, not having the terminal is pretty annoying.
Should not penalize legit users for some rogue users.
Deployed to https://lvupavicsmaster.ouranos.ca for testing.